Skip to content

fix(typescript): map included .ts files in step output - #5685

Open
luantaraschi wants to merge 2 commits into
codeceptjs:4.xfrom
luantaraschi:fix/ts-include-step-paths
Open

fix(typescript): map included .ts files in step output#5685
luantaraschi wants to merge 2 commits into
codeceptjs:4.xfrom
luantaraschi:fix/ts-include-step-paths

Conversation

@luantaraschi

Copy link
Copy Markdown

Motivation/Description of the PR

Resolves #5675.

@djyarber's observation that test files map correctly but included page objects do not comes down to two adjacent blocks in lib/container.js that do almost the same thing and disagree about where the result goes.

The helper block, around line 470, merges the transpile mapping into store.tsFileMapping. The include/support block, around line 889, merges the same shape of mapping into container.tsFileMapping only.

Step.line() in lib/step/base.js:156 reads store.tsFileMapping. So a step whose stack frame points into an included .ts module has no entry to match and keeps the .temp.mjs path, which by then has been deleted, hence the paths in the report.

Error stacks were never affected, which is why the migration guide's promise holds for failures: fixErrorStack() is handed the mapping object directly by the caller rather than reading it from store.

The include block now merges into store as well, mirroring the helper block.

Type of change

  • 🐛 Bug fix

Checklist:

  • Tests have been added
  • Documentation has been added (Run npm run docs) — N/A, no public API change
  • Lint checking (Run npm run lint)
  • Local tests are passed (Run npm test)

I want to be straight about the missing test. Exercising this needs a container built from a config with a TypeScript include, transpiled for real, and I did not get a harness for that working that I would trust. test/unit/utils/typescript_test.js drives transpileTypeScript directly and never touches the container. A test asserting Step.line() maps a path when store.tsFileMapping already holds the entry would pass before and after this change, so it would prove nothing.

What I did verify is the asymmetry itself: both blocks receive the same mapping from transpileTypeScript, only one writes to store, and store is what the reader uses. If you point me at the right fixture or harness for a config-level include I will add the regression test.

Unit suite on Windows: 758 passing / 11 failing, unchanged by this PR. Those 11 are pre-existing path assertions that expect POSIX paths and see a C: drive letter (utils_test.js, utils/trace_test.js).

Two adjacent blocks in container.js merge a transpile mapping after
compiling TypeScript. The helper block merges into store.tsFileMapping;
the include/support block merges only into container.tsFileMapping.

Step.line() reads store.tsFileMapping, so a step whose stack frame points
into an included page object had no entry to match and was printed with
the deleted .temp.mjs sibling instead of the .ts source. Error stacks were
unaffected because fixErrorStack() is handed the mapping directly.

The include block now merges into store as well, mirroring the helper
block two hundred lines above it.

Closes codeceptjs#5675
Copilot AI lite review requested due to automatic review settings August 7, 2026 12:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@djyarber

Copy link
Copy Markdown

Thanks for digging into this! Your read of the write/read mismatch is correct. Here’s a minimal browser-free config that reproduces it, with only this PR applied the Scenario Steps path is still wrong.

Working Repro

Browser-free, no network, ~1.1s. CodeceptJS 4.1.0 (latest on npm) with the one-line change from this PR hand-applied to lib/container.js. Same result on 4.0.8.

codecept.conf.js:

export const config = {
  tests: './tests/*Test.ts',
  helpers: { FakeHelper: { require: './fakeHelper.js' } },
  include: { fooPage: './pages/fooPage.ts' },
  require: ['tsx/cjs'],
  name: 'ts-step-paths-repro',
};

fakeHelper.js — avoids needing a browser at all:

import Helper from 'codeceptjs/lib/helper';

export default class FakeHelper extends Helper {
  doThing(label) { return label; }
  failNow(message) { throw new Error(message); }
}

pages/fooPage.ts:

export {};
const { I } = inject();

export default {
  open() {
    I.doThing('called from a .ts page object under include');
  },
};

tests/fooTest.ts — the failing step is what makes the Scenario Steps block print:

Feature('ts step paths');

Scenario('reports page object step path', ({ I, fooPage }) => {
  fooPage.open();
  I.failNow('deliberate failure so Scenario Steps is printed');
});

Result matrix

container.js (this PR) step/base.js ordering Step output
stock stock .temp.mjs
this PR stock .temp.mjs
stock fixed (below) .temp.mjs
this PR fixed (below) file://./pages/fooPage.ts (.ts, but file:// leftover)

Neither change is sufficient alone, both are necessary.

Defect

Even with this PR’s change applied (hand-patched into lib/container.js on 4.1.0), Scenario Steps still report .temp.mjs. Instrumenting Step.line() shows why:

[DEBUG] line to match: "at Object.open (file://./pages/fooPage.74609.ryob11cp.temp.mjs:4:11)"
[DEBUG] mapping present: true size: 1
[DEBUG]   entry mjs: "/abs/path/to/pages/fooPage.74609.ryob11cp.temp.mjs" -> matched: false

The mapping entry now exists, your change works, but the lookup still misses.

Step.line() shortens absolute paths to . before it looks them up in the mapping. The mapping’s keys are still absolute (transpileTypeScript stores them that way), so after shortening, line.includes(mjsFile) never finds a match.

Doing the mapping first, then shortening, seems to fix it:

   line() {
     const lines = this.stack.split('\n')
     if (lines[STACK_LINE]) {
-      let line = lines[STACK_LINE].trim()
-        .replace(store.codeceptDir || '', '.')
-        .trim()
+      let line = lines[STACK_LINE].trim()

-      // Map .temp.mjs back to original .ts files using container's tsFileMapping
+      // Map .temp.mjs back to original .ts files using container's tsFileMapping.
+      // The mapping holds absolute paths, so this must run before codeceptDir is
+      // shortened to '.', or the lookup can never match.
       const fileMapping = store.tsFileMapping
       if (line.includes('.temp.mjs') && fileMapping) {
         for (const [tsFile, mjsFile] of fileMapping.entries()) {
           if (line.includes(mjsFile)) {
             line = line.replace(mjsFile, tsFile)
             break
           }
         }
       }

-      return line
+      return line.replace(store.codeceptDir || '', '.').trim()
     }
     return ''
   }

Non-mapped step lines still shorten the same way, eg at Test.<anonymous> (./tests/fooTest.ts:7:5).

Even when mapping works, include steps still show file://./pages/fooPage.ts rather than ./pages/fooPage.ts, because shortening leaves the file:// scheme.

Line numbers are still wrong (separate issue)

#5675 also mentioned wrong line numbers, and that part is not fixed by either change. Green output reports ./pages/fooPage.ts:4:11, but I.doThing is at line 6, column 9, the numbers are still the transpiled .mjs's, off by the stripped export {} and blank line.

fileMapping is file-level only, so no path-substitution fix can recover line numbers. That needs line-preserving transpilation or real source maps. Worth splitting out rather than closing #5675 as fully fixed.

@luantaraschi

Copy link
Copy Markdown
Author

Thanks for the detailed repro and result matrix. You're right: this PR populates the mapping, but Step.line() shortens the absolute path before the lookup, so the absolute .temp.mjs key can never match.

I'll update this PR to apply the mapping before codeceptDir normalization, remove the leftover file:// prefix, and add the browser-free regression you outlined. I'll keep the incorrect line numbers separate because file-level mapping cannot recover source positions. This PR should only claim to restore the original .ts path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Scenario Steps show .temp.mjs instead of .ts for TypeScript include modules

3 participants